Redefine blocksizes #399
Merged
Redefine blocksizes #399
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release-1.0 #399 +/- ##
================================================
+ Coverage 0.00% 95.59% +95.59%
================================================
Files 16 18 +2
Lines 1462 1522 +60
================================================
+ Hits 0 1455 +1455
+ Misses 1462 67 -1395 ☔ View full report in Codecov by Sentry. |
|
@jishnub @dlfivefifty I think this is ready to review. I listed a few design questions to discuss in the first post. Seems like the downstream tests pass, I guess they weren't using the previous |
dlfivefifty
reviewed
May 10, 2024
Can't we infer the type of the block sizes from the axes? This would fix the printing.
I suspect using axes is better but this is an optimisation detail that can be changed later.
Sure |
|
Thanks for the feedback @dlfivefifty, I will update the PR this week reflecting your suggestions. |
|
@dlfivefifty I believe I've addressed all of your comments and the open issues I listed in the first post. |
dlfivefifty
approved these changes
May 14, 2024
dlfivefifty
added a commit
that referenced
this pull request
May 17, 2024
* Compact show for `BlockRange` (#248) * Compact show for BlockRange * update docstrings * don't specialize show for zero dim * fix missing io in print * missing show tests * show for BlockIndexRange * Bump version to v1.0.0-dev * Add `BlockedOneTo` as the axis type for a `BlockedArray` (#348) * Add BlockedOneTo * axes for AbstractBlockedUnitRange returns BlockedOneTo * Rewrite test using blockedrange instead of BlockedUnitRange * Update BlockedUnitRange docstring and add for BlockedOneTo/blockedrange * Show for BlockedOneTo * Blocklengths for OrdinalRange block sizes * Update docs * Return BlockedOneTo in indexing with BlockRange * Be less fussy in show tests * Require 1-based lasts in blockedrange * Disallow offset arrays in BlockedUnitRange * undo unnecessary doc change * Test conversions between BlockedOneTo and BlockedUnitRange * Reduce the number of convert methods * Remove axes1 specialization * Disallow offset block axes and blocks in BlockArray constructor * Remove unused axes method * Infinite broadcast tests (#383) * Specialize blockedrange BroadcastStyle for LazyArrayStyle (#384) * Specialize blockedrange BroadcastStyle for LazyArrayStyle * Add compat for LazyArrays * Define dataids for PseudoBlockArrays (#364) (#385) * Define dataids for PseudoBlockArrays (#364) * Don't use dataids of axes * Banded Matrix extension (#388) * Move BandedMatrices+BlockArrays code in BlockBandedMatrices to extension * Bump julia-actions/setup-julia from 1 to 2 (#387) Bumps [julia-actions/setup-julia](https://github.com/julia-actions/setup-julia) from 1 to 2. - [Release notes](https://github.com/julia-actions/setup-julia/releases) - [Commits](julia-actions/setup-julia@v1...v2) --- updated-dependencies: - dependency-name: julia-actions/setup-julia dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Move over blockbanded code * Add tests * Update Project.toml * add tests * Update Project.toml --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Generalize the element type of `BlockedUnitRange` (#337) * Allow more general BlockUnitRange element types * Restrict element type * Get tests passing * Fix some tests * Fix some doctests * Skip broken test in Julia v1.6 * Better support for unitful numbers * Fix tests * Stricter types in _BlockedUnitRange * Improve tests coverage * Allow non-Int eltypes in BlockedOneTo (#395) * Allow non-Int eltypes in BlockedOneTo * Specific constructors in BlockedOneTo * Restrict eltype to integers in BlockedOneTo constructors * Tests for construction from a Tuple * Move LazyArrays extension to LazyArrays.jl (#393) * Move LazyArrays extension to LazyArrays.jl * remove LazyArrays * Move over OneToCumsum This was type piracy in LazyBandedMatrices.jl * Update blockaxis.jl * move out InfiniteArrays.jl tests * Use FillArrays accumulate overloads (#397) * Bump julia-actions/setup-julia from 1 to 2 (#387) Bumps [julia-actions/setup-julia](https://github.com/julia-actions/setup-julia) from 1 to 2. - [Release notes](https://github.com/julia-actions/setup-julia/releases) - [Commits](julia-actions/setup-julia@v1...v2) --- updated-dependencies: - dependency-name: julia-actions/setup-julia dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Map imported names to correct parentmodules (#391) * Remove unused imported names (#392) * Don't import Base.Cartesian (#394) I don't think this is being used anymore * Use FillArrays accumulate * Bump julia-actions/cache from 1 to 2 (#398) Bumps [julia-actions/cache](https://github.com/julia-actions/cache) from 1 to 2. - [Release notes](https://github.com/julia-actions/cache/releases) - [Commits](julia-actions/cache@v1...v2) --- updated-dependencies: - dependency-name: julia-actions/cache dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update Project.toml * Update Project.toml * try running Pkg.update() --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jishnu Bhattacharya <[email protected]> * Redefine blocksizes (#399) * Redefine blocksizes * Revert change to docstring * Add tests, fix some tests, add docstring * Fix more tests * Add test Project.toml * Git ignore vim temp files * Fixes to test Project.toml * Another test Project.toml fix * Move code, change type design, better code coverage * Backwards compatibility. Fix doctest. * Fix tests * Redesign BlockSizes to be AbstractArray subtype * Rename PseudoBlockArray to BlockedArray (#401) * v1.0, add README * rename files * Update README.md --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Jishnu Bhattacharya <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Matt Fishman <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a first pass at #369, as discussed in #255.
To-do:
Some questions to discuss:
BlockSizesbe a subtype ofAbstractArray?In the latest, I didn't go that route and instead implemented the iterator interface. My reasoning was that it is not obvious whatEDIT: In the latest,eltypeshould be for arbitrary block arrays, and even within block arrays the types of the sizes of different blocks could be different.BlockSizes <: AbstractArraywith a concrete element type based oneltype.(axes(array)).It would be nice for it to print like an n-dimensional array but I don't know of a simple way to make use of AbstractArray printing without reaching into Julia Base internals. One strategy I was trying was to make another typeEDIT: Now it is handled automatically becauseBlockSizesArraywhich is likeBlockSizesbut is anAbstractArraysubtype, that works ok but then I don't know how to customize printing the type.BlockSizes <: AbstractArray.blocksizes(a::AbstractArray, d::Integer) = blocklengths(axes(a, d))? That would actually bring back the previous definition ofblocksizes(a::AbstractArray, d::Integer). EDIT: Implemented in the latest.